Skip to content

Conversation

@TimtyG
Copy link
Contributor

@TimtyG TimtyG commented Mar 22, 2025

Enabled a visible Close button in Editor Windows.

The Save and Cancel button is now only shown when an item is selected

Showcase

Edit (Projectile Window fixed)

allwindows.mp4

Resolves #2688

@TimtyG TimtyG marked this pull request as draft March 22, 2025 18:36
@TimtyG TimtyG marked this pull request as ready for review March 22, 2025 21:00
@WeylonSantana
Copy link
Contributor

WeylonSantana commented Mar 23, 2025

ok, some main changes to happen here
a - some windows allow minimizing, but not maximizing
-- animation, common event, npcs, spell, variable,
b - some windows allow maximizing (without minimizing)
-- quest,
c - some windows do not allow minimizing or maximizing
-- crafts, crafting tables,
d - some windows allow both
-- class, item, projectile, resources, shop, time
all allow closing, ok.

change 1 - all windows have to allow only the close button, (look at crafts, crafting table)

protected Button btnSave;
protected Button btnCancel;

protected void SetEditorButtons(Button saveButton, Button cancelButton)
Copy link
Contributor

@WeylonSantana WeylonSantana Mar 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the class fields are protected, that is, the children have access to the fields, this method is not necessary if the children can set them directly

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True, but I added SetEditorButtons just to make setup explicit and safer across forms, it's a small layer to avoid silent breakage and help with future logic reuse.

@WeylonSantana
Copy link
Contributor

WeylonSantana commented Mar 23, 2025

in all files - .Designer.cs, is where the code will assign these buttons for you, currently each designer file is overwriting them, remove of all .Designer.cs (is on end of every file)
image

@@ -0,0 +1,30 @@
using System.Windows.Forms;
Copy link
Contributor

@WeylonSantana WeylonSantana Mar 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and nothing prevents these two properties and the UpdateEditorButtons method from being in the EditorForm file, so take them there to avoid more unnecessary inheritances for something so trivial, delete this file and return the original files to : EditorForm

Copy link
Contributor Author

@TimtyG TimtyG Mar 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

EditorForm is handling lifecycle and hook, not UI controls. So I made the class to sit between , so we can isolate save/cancel for editor windows that use them. Every single form that inherits from it would get implicitly Save/Cancel logic. Over time, EditorForm will become a junk drawer for unrelated logic and new forms that don’t need Save/Cancel buttons are still carrying dead weight.

@WeylonSantana
Copy link
Contributor

finally, in each editor, look for references to btnSave and btnCancel, and check if there are any warnings about possible null references, if there are, check before if (btn != default)

@TimtyG
Copy link
Contributor Author

TimtyG commented Mar 23, 2025

in all files - .Designer.cs, is where the code will assign these buttons for you, currently each designer file is overwriting them, remove of all .Designer.cs (is on end of every file) image

This will remove the buttons from visual preview in designer.

@TimtyG
Copy link
Contributor Author

TimtyG commented Mar 23, 2025

ok, some main changes to happen here a - some windows allow minimizing, but not maximizing -- animation, common event, npcs, spell, variable, b - some windows allow maximizing (without minimizing) -- quest, c - some windows do not allow minimizing or maximizing -- crafts, crafting tables, d - some windows allow both -- class, item, projectile, resources, shop, time all allow closing, ok.

change 1 - all windows have to allow only the close button, (look at crafts, crafting table)

OK, done. All windows have only close button.

@TimtyG TimtyG requested a review from pandinocoder March 26, 2025 12:18
@pandinocoder pandinocoder removed their request for review March 26, 2025 13:57
@TimtyG
Copy link
Contributor Author

TimtyG commented Mar 26, 2025

@lodicolo could you please clarify what I messed up with the new commit?

@pandinocoder
Copy link
Member

I didn't say anything was wrong, GitHub doesn't allow me to do something I tried to do, and the result is that it says I removed my request for review.

Copy link
Member

@pandinocoder pandinocoder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm deferring to Weylon on this PR, I will approve when he has.

@WeylonSantana WeylonSantana merged commit 16bda43 into AscensionGameDev:main Mar 26, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: Cancel Button Missing in Animation Editor When No Item is Selected

3 participants